fix: gazelle failing on Windows with "panic: runtime error: invalid memory address or nil pointer dereference" #1872
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
…ter dereference" (bazelbuild#1871)" This reverts commit 9baee62.
…r: invalid memory address or nil pointer dereference" (bazelbuild#1871)
| @@ -126,7 +126,7 @@ type Configs map[string]*Config | |||
|
|
|||
| // ParentForPackage returns the parent Config for the given Bazel package. | |||
| func (c *Configs) ParentForPackage(pkg string) *Config { | |||
| dir := filepath.Dir(pkg) | |||
| dir := path.Dir(pkg) | |||
There was a problem hiding this comment.
Looking at the code, I am not sure it is working the way you think. path.Dir will just split at / if I understand things correctly and filepath.Path is much more sophisticated and I am surprised that this is not working correctly.
I don't have a Windows machine, but the code change looks suspicious.
There was a problem hiding this comment.
@aignas the same issue I have fixed here. My initial approach was to use cfgs[filepath.FromSlash(rel)] but a more targeted fix was proposed in this comment.
Basically, it fails because pythonconfig.go#ParentForPackage delegates to filepath.Dir(pkg) to replace the separator character "\" with "/", hence for a 2-levels deep path, e.g. rootDir\nestedDir1\nestedDir2, a parent is searched with "/" (rootDir/nestedDir1) but Configs contains a key with "\" (rootDir\nestedDir1).
There was a problem hiding this comment.
Ah, I see, thanks for the context, it was much needed here. Could you please rebase and add Changelog notes to the Changelog.md please?
EDIT: Would it be possible to add a test for this?
|
The PR looks good to me and I would love to merge it since we have cut a |
@aignas sorry for the delay, I'll do it by the end of this week. |
This pull request fixes
bazel run gazellefailing on Windows with "panic: runtime error: invalid memory address or nil pointer dereference" if there is a path with at least 2-levels deep directory.Fixes #1871